Type inference: Generalize typeConstraintBaseTypeMatch#22052
Conversation
4909d80 to
36724d8
Compare
36724d8 to
8cfb152
Compare
Rerun has been triggered: 3 restarted 🚀 |
typeConstraintBaseTypeMatchtypeConstraintBaseTypeMatch
There was a problem hiding this comment.
Pull request overview
This PR generalizes typeConstraintBaseTypeMatch to reuse the existing typeMatch infrastructure, enabling more general type inference through constraints. It also adds a Rust regression test demonstrating the newly inferred types and updates a consistency expected file to reflect shifted line numbers.
Changes:
- Refactor type-constraint-based inference to use
typeMatchmore broadly (and adjust related access/constraint plumbing). - Add a Rust test case covering inference through a chained trait constraint (
MyTrait2<T2>whereT2: MyTrait<T1>). - Update the Rust path-resolution consistency expected output to match new source locations.
Show a summary per file
| File | Description |
|---|---|
| shared/typeinference/codeql/typeinference/internal/TypeInference.qll | Generalizes constraint/type matching logic; (docs in the touched area need updating to reflect the new predicate signatures/semantics). |
| rust/ql/test/library-tests/type-inference/main.rs | Adds a new trait/impl/call chain to exercise improved inference. |
| rust/ql/test/library-tests/type-inference/CONSISTENCY/PathResolutionConsistency.expected | Updates expected locations after line shifts in main.rs. |
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Low
8cfb152 to
bfc37e5
Compare
Rerun has been triggered: 3 restarted 🚀 |
geoffw0
left a comment
There was a problem hiding this comment.
It's a bit of a fiddly diff that seems to ultimately be built on upgrading RelevantAccess.getTypeAt to use typeMatch. I can't see anything wrong with this.
DCA LGTM.
Copilot was keen that we add a "negative test case", but its suggestion doesn't compile, and I don't immediately see what would. I encourage you to consider such things in future to help catch mistakes.
The logic in
typeConstraintBaseTypeMatchwas previously hard-coded to only look at type parameters that could be directly matched via the type of an argument, but it is both more general and clean to use thetypeMatchpredicate (to whichtypeConstraintBaseTypeMatchalso itself contributes). I have added a test case that shows how this allows us to infer more types.DCA is uneventful, except there is an increase in
Nodes With Type At Length Limitforrendiation, but that will be reduced once #21795 is merged (this PR was extracted from that PR).